Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] Improve redis test suite #40816

Closed

Conversation

TheLevti
Copy link
Contributor

@TheLevti TheLevti commented Feb 4, 2022

This is a follow up of #40569, splitting the original pull request up. This pull request is about refining the existing redis test suite making it easy to test future changes related to redis.

What changed

  • Added data providers with named data sets to allow easy testing of different redis connection configurations.
  • All tests in all components that use redis are now using the new data providers to test all supported redis connection settings separately making it easy to pinpoint which redis setting causes an issue with an existing or new component.

If any test related to redis fails, it will now output the exact data set that caused the failure:

image

How to use

Now when you need to test something where redis is used, you can simply add the helper trait in the beginning like so:

use InteractsWithRedis;

Then to let a test run through all possible redis variations, add one of the following doc blocks:

/**
 * Runs this test with only two basic redis connections where one is using the predis driver and
 * the other the phpredis driver.
 *
 * @dataProvider redisConnectionDataProvider
 */
public function testSomethingRelatedToRedis($connection)
{
    $this->app['redis'] = $this->getRedisManager($connection);
    ...
}
/**
 * Runs this test with the basic connection set and in addition advanced connection configurations like
 * persistent connections, prefix, scan options, serialization, compression etc.
 *
 * @dataProvider extendedRedisConnectionDataProvider
 */
public function testSomethingRelatedToRedis($connection)
{
    $this->app['redis'] = $this->getRedisManager($connection);
    ...
}

To get a custom redis connection during a test you can also pass two additional parameters to the getRedisManager helper method where the second parameter is the driver and third parameter an connection config array:

$redisManager = $this->getRedisManager('predis_custom', 'predis', [
    'cluster' => false,
    'options' => [
        'prefix' => 'test_',
    ],
    'default' => [
        'url' => "redis://{$host}:{$port}",
        'database' => 5,
        'timeout' => 0.5,
    ],
]);

To properly clean up after every test provide the following method call in your tearDown method:

protected function tearDown(): void
{
    $this->tearDownRedis();

    parent::tearDown();
}

@TheLevti TheLevti changed the title [8.x] Improve test suite related to redis [8.x] Improve redis test suite Feb 4, 2022
@TheLevti TheLevti force-pushed the feature/improve-phpredis-test-suite branch 3 times, most recently from d46d276 to c8b7393 Compare February 4, 2022 20:30
- Added data providers to allow testing of different redis connection configurations.
- All tests using redis are now using data providers to test all relevant redis connection settings separately making it easy to pinpoint which redis setting causes an issue with a specific component.
@TheLevti TheLevti force-pushed the feature/improve-phpredis-test-suite branch from c8b7393 to 9f27990 Compare February 4, 2022 20:35
*/
private function initializeRedisManager($driver, $config)
{
$app = $this->app ?? new Application();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need new Application() here?

Copy link
Contributor Author

@TheLevti TheLevti Feb 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some Integration tests do not use the Laravel test case and there we need to create one as the class prop is not set in that case.

@taylorotwell
Copy link
Member

Can you send this to 9.x branch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants